WELD-2764 Method invokers, first impl #2864
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
manovotn
force-pushed
the
invokableMethods
branch
2 times, most recently
from
0ba2566
to
313d464
Compare
manovotn
force-pushed
the
invokableMethods
branch
from
bdbffce
to
eb14f9c
Compare
manovotn
force-pushed
the
invokableMethods
branch
2 times, most recently
from
e3ddebe
to
b8fa638
Compare
An instance of `CleanupActions` must exist for each _invocation_, not for each _invoker_. This requires some creative juggling of method handle arguments, which is what this commit does.
…ing the ultimate invocation
This currently doesn't help anything, but it should be possible to optimize out `CleanupActions` instantiation completely when we know that it is never used.
This required moving the invoker construction logic from `InvokerImpl` into the `AbstractInvokerBuilder`, which is arguably a better place anyway. The `InvokerImpl` looks fairly minimal now, as it should.
manovotn
force-pushed
the
invokableMethods
branch
from
3c0897c
to
9d4538c
Compare
…now be created from any method
|
Added changes to support recent adjustment in CDI PR - removal of This also reduces the volume of this PR from 96 changes files to just 68 :) |
|
I guess all of the in-container tests fail because we aren't patching the CDI JAR in WFLY dist; will take a closer look on Mon as to whether we can improve that until we have WFLY version which consumes the expected artifact. |
…n WFLY. Use this in CI jobs.
manovotn
changed the title
|
I've created a follow up JIRA issue capturing leftover TODOs from this one and targetting Alpha2. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
JIRA - WELD-2764
Draft for the prototype of invokeable methods in Weld.
CI failures are expected due toSNAPSHOTdeps.CI should now be fine , there are no longer any snapshot dependencies and there is added profile that allows CDI and interceptors API patching in WFLY. All tests should pass.
The impl is based on Java
MethodHandles.How to try out:
mvn clean install) the CDI branch with invokeable methods API proposalmvn clean install -DskipTests)5.1.2-SNAPSHOTwith draft of the implLooking at performance:
The implementation uses
MethodHandlesand as such is very efficient. The difference in direct method invocation versus invocation viaInvokerbecomes negligible the moment method does anything non-trivial (basically anything else than returning a constant). Currently, I have been playing with some microbenchmarks using JMH to test this. A WIP can be seen over in this repo and branch. If you want to run that:invokableMethodsbranchmvn clean install./run-benchmarks.sh -v "5.1.2-SNAPSHOT" -b "InvokableMethodBenchmarkNoLookup InvokableMethodBenchmarkSimpleBeanInvocation InvokableMethodBenchmarkLookup"Known TODOs:
Support for build compatible implementations (currently only works via portable extensions)Support exception transformers returning arbitrary value (which is something method handles don't allow out of the box)Properly support lookup with qualifiers@Invokablewe now need to add verification that happens when you attempt to create an invoker for any given method; for instance validate that the method is non-private